[#885] Bound the create of the tree catalog, and say when an engine has no bound to give - #1003

Part of #885, on top of #936 (merged as 21d03d5): withDdlLockBound() is that PR's code. Rebased onto master after that merge - the diff here is one commit, two files.

Problem

#936 bounded the DDL of this backend by putting withDdlLockBound() on the two paths its DDL goes through - commitStatement(sql, ddl==true) and the drop loop of a removed backend. There is a third, and it is neither of them.

createCatalogTable() issues its create table on the catalog's own connection, before openTree() enrols the tree it is opening. That statement:

• goes through neither funnel, so [#885] Bound the wait of a JDBC DDL for a lock another session holds #936 leaves it as unbounded as it was;
• is StatementBound.BULK, so it carries no query timeout by design ([#877] Bound a statement of the JDBC backend by the class of the work it belongs to #882);
• and is unbounded at the socket too, because [#885] Give a connection of the JDBC pool a read bound of its own, and take it off for a statement that carries none #934 lifts the standing read bound for the length of a statement of an unbounded class.

So after #934 and #936 it is the one statement of this backend with no bound of any kind. Where it runs makes that worse rather than better: it is on the open path, and it runs inside synchronized (catalogLock) - a wait there parks every other thread of the storage that wants the catalog, not only the one that queued.

Nor does that wait need an exotic database. The method's own comment names the case: "a table that turned up between the lookup and this statement is what was wanted, whoever made it" - an offline tool beside a running server, the pair #888 is about. On postgres a second session creating that same table inside a transaction it has not committed makes this one wait on that transaction, lock_timeout being 0 by default; a lock held on the schema does the same.

It arrived after both were written: createCatalogTable() came with #893, which merged the same morning #934 did, and #936 took it in through its merge of master without the bound reaching it.

Change

1. The create of the catalog table runs under withDdlLockBound(), like every other DDL of this backend. The commit is inside the bound, because on postgres it is that commit which ends the transaction a set local belongs to.

Everything the bound leans on already works on this connection: physical() passes a raw connection through, dialectOf() reads the driver name off it, and restoreDdlLockBound() already asks instanceof CachedConnection before keeping a connection out of a pool. What did not fit is what two comments claim, this being the one connection reaching that code which was never in the pool: the javadoc of withDdlLockBound() and the "left behind" warning of restoreDdlLockBound() now say what becomes of the catalog's own - it is closed with the write that opened it, so a setting left on it reaches the rest of that write and nothing after it.

A lock this create gives up on reaches the operator named: gaveUpOnTheLock() rewrites it, the existing catch carries it into the StorageRuntimeException saying which table the backend wanted and why, and the property is in that chain. Unbounded, it arrived as a bare 55P03 inside a message about privileges - which is the wrong thing to go and check.

2. An engine this backend knows no lock setting for is reported, once. The early return of withDdlLockBound() had three reasons behind one condition; they are three conditions now, and only the middle one speaks:

• seconds<=0 - the deployment asked for no bound, and there is nothing to say;
• dialect==null - reported, naming the class of the driver;
• ddlLockBoundSql(seconds,null)==null - oracle, left alone deliberately and documented as such.

dialectOf() keys on the driver class name, so this is not the engine of an exotic database alone: MariaDB Connector/J - or a Percona- or Aurora-branded driver - against a live MySQL answers null here, and that is a session whose lock_wait_timeout is a year, which is the wait this bound exists to end. Leaving the bound off there stays, and testAnEngineThisBackendDoesNotKnowIsLeftAlone still pins it: untested SQL is no thing to send a database on the path a backend opens by. Only the silence goes, for the reason the strict parsing of the property exists - a deployment that asked for a bound is never quietly left with none. reportUnknownDialect() in CachedConnection cannot cover this one: it keys on the url, which such a driver reads as a mysql one, and it speaks of the connect bound and pool.timeout. The driver is named rather than the url, since the driver is what this reads and what a deployment would change - and a url carries the password of the account the backend works as.

The javadoc of ddl.lock.timeout now names this case beside the oracle exemption, that being what an operator reads after meeting a DDL which waited anyway.

Testing

JDBCDdlLockBoundTestCase gains six cases: what postgres and what mysql are told around the create of the catalog table (the readback and the value given back among them), the lock it gave up on naming the property out to the caller, and the three placements of the report - the unknown engine, the bound nobody asked for, and the oracle left alone on purpose.

mvn -o -pl opendj-server-legacy -am -Pprecommit -Dfailsafe.failIfNoSpecifiedTests=false verify
    -Dit.test='JDBCDdlLockBoundTestCase,CatalogConnectionTestCase,CachedConnectionTestCase,JDBCStatementBoundTestCase,JDBCStorageRetryTest,StampConnectionTestCase'

306 tests, 0 failures, 0 errors, 0 skips, on the rebase onto master (21d03d5) - the two cases the last commit of #936 added to JDBCDdlLockBoundTestCase among them.

Passing is not enough on its own, so every guard was built with its defect put back:

One thing the new cases needed, and the suite is the better for it: the storage of a case now answers newStampConnection() itself. Reaching openTree() means reaching the stamp of #866, whose connect would otherwise go to DriverManager - which registers every JDBC driver on the classpath. This suite needs no database and should touch none: reuseForks=false keeps that from reaching another class of this build, but it reaches one in an IDE, where CatalogConnectionTestCase needs its own probe driver registered ahead of pgjdbc.

Out of scope

• Oracle, for the reason [#885] Bound the wait of a JDBC DDL for a lock another session holds #936 gives: ddl_lock_timeout gives up at once by default, and ours would only loosen it.
• The 0 default of read.timeout ([#885] Give a connection of the JDBC pool a read bound of its own, and take it off for a statement that carries none #934) and the unbounded bulk.timeout ([#877] Bound a statement of the JDBC backend by the class of the work it belongs to #882), both shipped deliberately.
• A container case for this one. Holding a create table of the same name in an uncommitted transaction of a second session is postgres alone - mysql and oracle commit their DDL as they issue it - so a case in jdbc/TestCase would be a case about one engine wearing four engines' clothes. What each of the four is told around this create is pinned by the mock cases instead.